-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: split gw ctm deployer #4615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| .await?; | ||
|
|
||
| let l1_da_validator_addr = get_l1_da_validator(&chain_config) | ||
| let l1_da_validator_addr = get_l1_da_validator(&chain_config, args.validium_type.clone()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it can be discoverable.
let's refactor the script as the next step
zkstack_cli/crates/zkstack/src/commands/chain/gateway/migrate_to_gateway.rs
Outdated
Show resolved
Hide resolved
| L1BatchCommitmentMode::Validium => { | ||
| let general_config = chain_config.get_general_config().await?; | ||
| match general_config.da_client_type().as_deref() { | ||
| // Use CLI param if provided, otherwise read from general config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to convert to string, for converting it to contracts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'll still need to match by string for the sake of chain_config.get_general_config().await?.da_client_type() working, I can only reduce code repetition by giving some common method to convert ValidiumTypeInternal to a string
| } | ||
| } else { | ||
| let da_client_type = chain_config.get_general_config().await?.da_client_type(); | ||
| // For Validium, use CLI param if provided, otherwise read from general config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to convert to string, for converting it to contracts
|
CI is not worse than before, merging |
What ❔
For the purposes of zksync os integration, we had to split GatewayCTMDeployer into multiple contracts.
Also, we had to adapt some of the scripts to work fine with the latest zksync os workflows
Why ❔
Is this a breaking change?
Operational changes
Checklist
zkstack dev fmtandzkstack dev lint.